Fixups for 2023-11-03 SDK #101
Conversation
- `HttpResponse` fields to be vanilla `*http.Response` - Embed all response structs for continuity - General tidy up, remove shadowed vars, improve error handling
There was a problem hiding this comment.
Thanks @manicminer - I spotted a few things we could fix up but mostly this looks good to me 👍
| // TODO commenting these out to see if they're really needed (net/http should do this) (@manicminer) | ||
| //req.Body = io.NopCloser(bytes.NewReader(input.Content)) | ||
| //req.ContentLength = int64(len(input.Content)) |
There was a problem hiding this comment.
these might be required, I'm seeing this error when running the tests now:
Error putting page: executing request: unexpected status 403 with response: <?xml version="1.0" encoding="utf-8"?><Error><Code>AuthenticationFailed</Code><Message>Server failed to authenticate the request. Make sure the value of Authorization header is formed correctly including the signature.
There was a problem hiding this comment.
Yeah it seems so. There's probably an underlying fix to be made somewhere but this will do as a workaround for now :)
| @@ -3,13 +3,13 @@ package blobs | |||
| import ( | |||
| "context" | |||
| "fmt" | |||
| "github.com/tombuildsstuff/giovanni/storage/internal/metadata" | |||
There was a problem hiding this comment.
this import has gotten out of order
| } | ||
|
|
||
| if v := resp.HttpResponse.Header.Get("Content-Length"); v != "" { | ||
| if v := resp.Header.Get("Content-Length"); v != "" { | ||
| i, innerErr := strconv.Atoi(v) | ||
| if innerErr != nil { | ||
| err = fmt.Errorf("error parsing %q as an integer: %s", v, innerErr) |
There was a problem hiding this comment.
should we update these errors too to mention which header it is for consistency?
| @@ -3,13 +3,13 @@ package blobs | |||
| import ( | |||
| "context" | |||
| "fmt" | |||
| "github.com/tombuildsstuff/giovanni/storage/internal/metadata" | |||
There was a problem hiding this comment.
import here needs sorted
| @@ -3,14 +3,13 @@ package filesystems | |||
| import ( | |||
| "context" | |||
| "fmt" | |||
| "github.com/hashicorp/go-azure-sdk/sdk/client" | |||
There was a problem hiding this comment.
imports need sorted here
| @@ -3,32 +3,35 @@ package files | |||
| import ( | |||
| "context" | |||
| "fmt" | |||
| "github.com/tombuildsstuff/giovanni/storage/internal/metadata" | |||
There was a problem hiding this comment.
imports need sorted here
| @@ -3,16 +3,16 @@ package files | |||
| import ( | |||
| "context" | |||
| "fmt" | |||
| "github.com/tombuildsstuff/giovanni/storage/internal/metadata" | |||
There was a problem hiding this comment.
imports need sorting here
| @@ -3,16 +3,16 @@ package shares | |||
| import ( | |||
| "context" | |||
| "fmt" | |||
| "github.com/tombuildsstuff/giovanni/storage/internal/metadata" | |||
There was a problem hiding this comment.
imports need sorted here
| @@ -3,16 +3,16 @@ package shares | |||
| import ( | |||
| "context" | |||
| "fmt" | |||
| "github.com/tombuildsstuff/giovanni/storage/internal/metadata" | |||
There was a problem hiding this comment.
imports need sorted here
|
Thanks @catriona-m! I've fixed up those and re-added the Content-Length workaround in the blobs client if you could take another look? |
HttpResponsefields to be vanilla*http.ResponseGetPropertiesInput{}.Actioninstorage/2023-11-03/datalakestore/paths/properties_get.go